feat: Add todo validation for attempt completion #5460
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Add validateTodosForCompletion function to check for incomplete todos - Modify attemptCompletionTool to validate todos before allowing completion - Block completion when todos have 'pending' or 'in_progress' status - Provide detailed error messages showing which todos are incomplete - Add comprehensive test coverage for validation logic - Maintain existing functionality when no todos exist Resolves Slack request to prevent completion with incomplete todos
dosubot
bot
added
size:L
|
✅ No security or compliance issues detected. Reviewed everything up to 97987f3. Security Overview
Detected Code Changes
Reply to this PR with |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
|
|
||
| // If there are any incomplete todos, block completion | ||
| if (pendingTodos.length > 0 || inProgressTodos.length > 0) { | ||
| let errorMessage = "Cannot attempt completion while there are incomplete todos:\n\n" |
Contributor
There was a problem hiding this comment.
User-facing error messages (e.g. in validateTodosForCompletion) are hardcoded. It is recommended to use the internationalization function to support translations consistently.
This comment was generated because it violated a code review rule: irule_C0ez7Rji6ANcGkkX.
hannesrudolph
added
the
Issue/PR - Triage
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR implements the requested feature to return an error if the model tries to attempt completion with incomplete todos.
Changes
Testing
Resolves the Slack mention request from @U08ELT3D32A to prevent completion with incomplete todos.
Important
Adds validation to block task completion if todos are incomplete, with tests and error messages.
validateTodosForCompletioninupdateTodoListTool.tsto check todos' completion status.attemptCompletionToolinattemptCompletionTool.tsto usevalidateTodosForCompletionbefore allowing task completion.pendingorin_progress, providing detailed error messages.validateTodosForCompletioninupdateTodoListTool.spec.ts.attemptCompletionToolinattemptCompletionTool.spec.tsto ensure completion is blocked with incomplete todos.This description was created by
for 97987f3. You can customize this summary. It will automatically update as commits are pushed.